-
Notifications
You must be signed in to change notification settings - Fork 8
Introduce network mocking to the toolkit #969
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Introduce network mocking to the toolkit #969
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kaushikrw - looks good. I suggest to merge this straight into v.next
though. Once on v.next
, @rom14514 can continue the work to consolidate the test utils in a shared library etc. At some point this would also be merged into v.next and if you are still working on this feature branch at that point, you can rebase.
Would that work?
I may have branched off from the feature branch.. so this might also include changes from the feature branch. Is that okay? Alternatively, I can create another PR to merge the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, approving so this could move forward but I do have questions to guide the follow-up work
|
||
// Grant camera permission for barcode scanning | ||
@get:Rule | ||
val runtimePermissionRule: GrantPermissionRule = | ||
GrantPermissionRule.grant(Manifest.permission.CAMERA) | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I am understanding this correctly, then the GrantTestPermissions
plugin doesn't actually need to grant the camera permission before running the tests, because the test can request it. I have some questions about this though:
Could we grant the storage permissions usingI investigated this and it's not possibleGrantPermissionRule
? If so, could we add that as aBeforeAfterAction
once we have that available to the toolkit (soon)?- If we can't do the above, could we keep the Camera permission granted at this level? This would be helpful because it would mean we could share the grant permissions task from the SDK plugin without modifications
- When we use
GrantPermissionRule
, we don't need to worry about the manifest? I'm guessing that is the case because I don't see it in the docs and you are just now adding the permissions to the manifest
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If we can't do the above, could we keep the Camera permission granted at this level? This would be helpful because it would mean we could share the grant permissions task from the SDK plugin without modifications.
Yeah, the camera permission can stay within the test. I just moved it out because the having all the permissions granted by the plugin made sense.
When we use GrantPermissionRule, we don't need to worry about the manifest? I'm guessing that is the case because I don't see it in the docs and you are just now adding the permissions to the manifest
I believe so according to the doc. But the manage storage permission definitely need the manifest.
Ok, in that case, I think the options are:
Whatever works best for you... |
* bump hilt version * remove validation errors filtering (#762) * updated validation behavior (#787) * bump androidXCamera and ksp * `Forms` : Fix how Attachment states are created (#801) * update how attachment states are created * remove onExpressionsEvaluated * Update gradle.properties * `FeatureForm` : initial support for UtilityAssociations (#776) * `Forms` : New Attachments UI (#825) * fix compilation error due to missing parameter * `Forms`: Microapp updates (#834) * enable and fix edge-to-edge * use safedrawing * fix select feature dialog height * add loading callout when identifying layers * `FeatureForm` : UN Associations enhancements (#783) * `FeatureForm` : Add "Delete" Associations UI (#855) * `FeatureForm` : Add UtilityAssociationsFormElement UI tests (#863) * `FeatureForm` : Update Association Display UI (#889) * `FeatureForm` : Provide navigation blocking (#909) * `FeatureForm` : Re-introduce ValidationErrorVisibility (#910) * `FeatureForm` : Update attachment size limits (#921) * `FeatureForm`: Update UN Association Display Spec (#934) * update login view model to use list of oauth user configurations (#932) * `FeatureForm`: Micro-app - Update how thumbnails are rendered (#959) * bump sdk version to 300.0 * `FeatureForm` : Remove deprecated api (#965) * `FeatureForm`: Microapp add folder browsing (#964) * `FeatureForm` : Micro-app datastore updates (#968) * `FeatureForm` : Add delete UNA implementation (#978) * add navigate back when last association is deleted (#983) * add test case 12.6 (#982) * `FeatureForm` : Honor UNA is editable and minor refactor (#986) * `FeatureForm` : UNA Add Implementation (Feature 1) (#988) * Add filter/search functionality (#991) * refactor navigation to support nested destinations * add paging, symbology display for features * change feature list title * add association creation * add asset type selection * add final screens * optimize imports and extract strings * update association creation logic * extract strings * updated ignored api classes * Add filter/search functionality * revert logic to determine query result size from paging params * add ignore for "com.arcgismaps.toolkit.featureforms.internal.utils.ComposableSingletons\$SearchBarKt" * remove paging * CR : Puneet * rename routes * rename more routes * move filtering logic to the composable from viewModel * update SearchBar UI * abstract filter logic to viewmodel * update doc * address code review comments * update padding * don't clear sources filter in selectSource --------- Co-authored-by: Kaushik Meesala <[email protected]> * `FeatureForm` : UI updates for the UNA add impl (#993) * `FeatureForm` : Updates to AddAssociationFromSourceViewModel.kt (#996) * Introduce network mocking to the toolkit (#969) --------- Co-authored-by: Erick Lopez Solis <[email protected]> Co-authored-by: Puneet Prakash <[email protected]>
Related to issue: #kotlin/6416
Description:
This PR introduces the framework for mocking network responses for any tests in the toolkit.
Summary of changes:
mockingjay
library which is used for mocking network data.GrantTestPermission
which will grant the required permissions to the test app. This is used for granting elevated permissions such asMANAGE_EXTERNAL_STORAGE
which is necessary for reading the mocked response files stored in the root of the storage on the test device.FeatureFormTests
.Note : This is dependent on the companion sdk PR in the linked issue.
Pre-merge Checklist